[codex] Refactor app maturity and extensibility - #1
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the desktop application toward clearer architectural boundaries (composition root, runtime path management, application-owned sweep runner/services, and UI decomposition), while adding packaging/CI metadata, expanded documentation, and a broader automated test suite to keep the refactor safe and reproducible.
Changes:
- Introduces a desktop composition root (
app/bootstrap.py) and centralized runtime paths (app/runtime/paths.py), simplifyingmain.pyand wiring. - Moves sweep execution concerns into application services (planner/configurator/acquirer/measurement/calibration + threaded
SweepTaskRunner) and splits Tk UI responsibilities (control panel + UI event handler). - Adds packaging + CI + docs + boundary tests and expands test coverage for settings/IO/sweep logic.
Reviewed changes
Copilot reviewed 72 out of 75 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| UserGuide/README.md | Points reviewers/operators to the Markdown operator guide for repo-friendly reading. |
| tests/test_sweep_task_runner.py | Adds coverage for threaded sweep execution, auto-save, and cleanup warning emission. |
| tests/test_sweep_planner_service.py | Adds tests for sweep planning and sampling window calculation. |
| tests/test_start_sweep_use_case.py | Updates fake port surface to include output_off for new shutdown behavior. |
| tests/test_settings_serializer.py | Verifies settings (de)serialization round-trip for schema v1. |
| tests/test_point_measurement_services.py | Tests point measurement math and calibration application behavior. |
| tests/test_measurement_io.py | Adds MAT/CSV export-load round trip tests and validates demo MAT loading. |
| tests/test_auto_range_policy.py | Adds tests for the new auto-range decision policy. |
| tests/test_architecture_boundaries.py | Adds AST-based layering tests to prevent dependency drift between domain/app/presentation/infrastructure. |
| tests/test_app_paths.py | Tests derivation of runtime paths from a root directory. |
| src/mapping.py | Converts legacy mapping.py into a compatibility re-export to app.shared.mapping. |
| src/main.py | Slims entrypoint to delegate desktop startup to app.bootstrap.run_desktop_app(). |
| src/equips.py | Reframes as a legacy vendor layer (docstring update) to be wrapped by infrastructure adapters. |
| src/cvtTools.py | Converts legacy cvtTools.py into a compatibility re-export to app.shared.cvt_tools. |
| src/app/shared/mapping.py | Centralizes UI strings/defaults and model labels in a shared module. |
| src/app/shared/cvt_tools.py | Centralizes parsing/conversion helpers in a shared module. |
| src/app/shared/init.py | Exposes shared utilities via an explicit module API. |
| src/app/runtime/paths.py | Adds AppPaths for centralized config/data/runtime path derivation. |
| src/app/runtime/init.py | Introduces runtime helper package marker/docstring. |
| src/app/presentation/tk/view_model.py | Updates mapping import to app.shared.mapping. |
| src/app/presentation/tk/ui_event_handler.py | Adds a dedicated UI event handler for application events and plot refresh. |
| src/app/presentation/tk/sweep_task_runner.py | Adds a presentation-layer compatibility re-export for SweepTaskRunner. |
| src/app/presentation/tk/plot_widget.py | Updates imports to shared CvtTools/Mapping. |
| src/app/presentation/tk/mapper.py | Updates import to shared CvtTools and continues VM↔settings mapping. |
| src/app/presentation/tk/controller.py | Refactors controller to use SweepTaskRunner, UiEventHandler, AppPaths, and address caching for monitoring. |
| src/app/presentation/tk/control_panel.py | Splits UI controls into a dedicated ControlPanel to keep AppWindow manageable. |
| src/app/presentation/tk/app_window.py | Simplifies window composition by embedding ControlPanel and aliasing key widgets. |
| src/app/infrastructure/persistence/settings_serializer.py | Introduces a dedicated settings serializer, decoupled from the repository. |
| src/app/infrastructure/persistence/settings_repo_json.py | Refactors JSON settings repository to use FileStore, defaults factory, and serializer. |
| src/app/infrastructure/persistence/settings_defaults.py | Adds a single source of truth for default settings creation. |
| src/app/infrastructure/persistence/repository_ports.py | Converts legacy repo port definitions into compatibility re-exports from application ports. |
| src/app/infrastructure/persistence/measurement_repo_mat_csv.py | Refactors measurement repo to delegate to exporter/loader helpers. |
| src/app/infrastructure/persistence/measurement_loader.py | Adds a standalone loader for MAT/CSV measurement inputs with phase alignment handling. |
| src/app/infrastructure/persistence/measurement_exporter.py | Adds a standalone exporter for MAT/CSV/TXT plus optional plot outputs. |
| src/app/infrastructure/persistence/file_store.py | Adds a small file abstraction for repository IO (mkdir + read/write). |
| src/app/infrastructure/instruments/vendor_gateway.py | Centralizes vendor instrument construction behind a single import point. |
| src/app/infrastructure/instruments/ports.py | Converts legacy instrument port definitions into compatibility re-exports from application ports. |
| src/app/infrastructure/instruments/osc_adapter.py | Switches instrument instantiation to the shared vendor gateway. |
| src/app/infrastructure/instruments/equips_factory.py | Uses application InstrumentPorts and constructs ports via updated adapters. |
| src/app/infrastructure/instruments/awg_adapter.py | Adds explicit output_off and routes vendor instantiation through the gateway. |
| src/app/domain/exporters.py | Changes export shaping to preserve row alignment for sparse phase/complex values. |
| src/app/domain/calibration.py | Hardens reference application against zero/near-zero reference values. |
| src/app/domain/auto_range.py | Extracts auto-range logic into a policy object with a structured decision output. |
| src/app/bootstrap.py | Adds a desktop composition root to wire ports, repos, use cases, controller, and paths. |
| src/app/application/use_cases/start_sweep.py | Refactors start-sweep orchestration into application services (planner/configurator/acquirer/measurement/calibration). |
| src/app/application/use_cases/settings_use_case.py | Updates use case to depend on application persistence ports. |
| src/app/application/use_cases/save_measurement.py | Updates use case to depend on application persistence ports. |
| src/app/application/use_cases/load_reference.py | Updates use case to depend on application persistence ports. |
| src/app/application/use_cases/load_measurement.py | Updates use case to depend on application persistence ports. |
| src/app/application/services/sweep/waveform_acquirer.py | Extracts waveform acquisition + auto-range application and warning generation. |
| src/app/application/services/sweep/point_measurement_service.py | Extracts per-point measurement computation from acquired waveforms. |
| src/app/application/services/sweep/planner.py | Introduces a planner wrapper over sweep engine + sampling window computation. |
| src/app/application/services/sweep/models.py | Adds sweep service DTOs (plan, acquired data, warnings). |
| src/app/application/services/sweep/instrument_configurator.py | Extracts instrument configuration logic out of the use case. |
| src/app/application/services/sweep/calibration_applier.py | Extracts calibration/reference application logic into a service. |
| src/app/application/services/sweep/init.py | Exposes sweep services via a package API. |
| src/app/application/services/sweep_task_runner.py | Adds an application-owned threaded runner for sweeps, autosave, and cleanup warnings. |
| src/app/application/services/connection_monitor.py | Updates to use instrument ports from application layer and improves stop/join behavior. |
| src/app/application/ports/persistence.py | Introduces application-layer repository Protocols (ports). |
| src/app/application/ports/instruments.py | Introduces application-layer instrument ports + ports factory typing. |
| src/app/application/ports/init.py | Re-exports ports for simplified imports. |
| requirements.txt | Tightens runtime dependencies to the actual app requirements with minimum versions. |
| requirements-dev.txt | Adds dev dependency set (ruff) layered on runtime requirements. |
| README.md | Expands architecture, usage, safety, docs links, and reproducible workflow guidance. |
| pyproject.toml | Adds setuptools packaging metadata, console script, and optional dependency groups. |
| docs/safety.md | Adds explicit operational/safety notes and shutdown/stop behavior documentation. |
| docs/operator_guide.md | Adds a repository-friendly operator workflow guide. |
| docs/images/README.md | Documents screenshot capture expectations for docs/portfolio artifacts. |
| docs/extending.md | Documents extension seams: composition root, paths, new instruments, persistence, UI fields, boundary tests. |
| docs/case_study.md | Adds a narrative case study describing constraints, architecture, testing strategy, and outcomes. |
| docs/architecture.md | Updates architecture doc with diagrams, dependency rules, and updated event/task-runner flow. |
| demo_data/README.md | Documents demo MAT files and how to load them in the app. |
| .gitignore | Adds standard Python/tooling ignores plus runtime output directories. |
| .github/workflows/ci.yml | Adds CI workflow running unit tests on Python 3.11 with repo PYTHONPATH. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
200
to
210
| def on_close(self) -> None: | ||
| self._monitor.stop() | ||
| if self._stop_use_case is not None: | ||
| self._stop_use_case.stop() | ||
|
|
||
| try: | ||
| settings = vm_to_settings(self.vm) | ||
| self.settings_use_case.save(settings) | ||
| except Exception: | ||
| pass | ||
|
|
||
| self._close_ports() | ||
| self._task_runner.shutdown() | ||
| self.window.destroy() | ||
|
|
Comment on lines
+90
to
+98
| if not result.is_empty and cmd.settings.auto_save_data: | ||
| target = SaveTarget( | ||
| base_path=self._auto_save_dir / "measurement", | ||
| include_timestamp=True, | ||
| figures={}, | ||
| ) | ||
| self._save_measurement_use_case.execute(result=result, settings=cmd.settings, target=target) | ||
| except Exception as exc: # noqa: BLE001 | ||
| self._emitter.emit(SweepFailed(error_code="SWEEP_THREAD", message=str(exc))) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR packages the Auto-Load-off-Test maturity and extensibility work into a feature branch.
It adds reproducible project metadata and CI, expands reviewer-facing docs, and refactors the app structure around clearer extension seams: a desktop composition root, centralized runtime paths, application-owned sweep task runner, application-level instrument port factory types, and a split Tk control panel.
Key changes
.gitignore,pyproject.toml,requirements-dev.txt, and GitHub Actions CI.app/bootstrap.pyand runtime paths intoapp/runtime/paths.py.SweepTaskRunnerout of Tk presentation intoapp/application/services/sweep_task_runner.py.InstrumentPortsand factory typing into the application port layer.AppWindowto keep UI growth manageable.Validation
PYTHONPATH=src .venv/bin/python -m unittest discover -s tests-> 28 tests OKPYTHONPATH=src python3 -m unittest discover -s tests-> 28 tests OKPYTHONPATH=src .venv/bin/python -m compileall -q src tests-> OKpython3 -c 'import pathlib, tomllib; tomllib.loads(pathlib.Path("pyproject.toml").read_text())'-> OKgit diff --check-> OKapp.bootstrap.build_desktop_app()-> OKNotes
The current branch also includes the existing local commit
d7393d9that was already ahead oforigin/mainbefore this work.